Skip to content

Conversation

@prdoyle
Copy link
Contributor

@prdoyle prdoyle commented May 29, 2025

A few refactorings to hide some more implementation details of entitlements.

@prdoyle prdoyle self-assigned this May 29, 2025
@prdoyle prdoyle requested a review from a team as a code owner May 29, 2025 19:44
@prdoyle prdoyle added >refactoring auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure labels May 29, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label May 29, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


requires static org.elasticsearch.entitlement.bridge; // At runtime, this will be in java.base

exports org.elasticsearch.entitlement.runtime.api;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Should this drop to the lines with the other runtime exports?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the one we proudly export. The others are not supposed to be public, but they currently are of necessity, so I pulled them down into their own section with a TODO.


private static final Module ENTITLEMENTS_MODULE = PolicyManager.class.getModule();

public static InitializeArgs initializeArgs;
Copy link
Contributor

@jdconrad jdconrad May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InitializationArgs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured they are the arguments for the initialize method. Not the best name I ever invented.

checker = EntitlementInitialization.initChecker(inst, createPolicyManager(initializeArgs.pathLookup()));
}

public record InitializeArgs(PathLookup pathLookup) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestInitializationArgs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto. The problem this solves is that we can't pass arguments directly to the initialize method.

@prdoyle prdoyle merged commit 9e40dc4 into elastic:main May 30, 2025
18 checks passed
prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request May 30, 2025
* Rename and encapsulate InitializeArgs

* Move ElasticsearchEntitlementChecker out of api package.

It's an implementation detail that doesn't need to be exposed to the rest of
the system.

* Stub TestPathLookup (not yet implemented)
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.19

elasticsearchmachine pushed a commit that referenced this pull request May 30, 2025
* Encapsulate entitlements (#128637)

* Rename and encapsulate InitializeArgs

* Move ElasticsearchEntitlementChecker out of api package.

It's an implementation detail that doesn't need to be exposed to the rest of
the system.

* Stub TestPathLookup (not yet implemented)

* Move entitlement checkers to policy package.

Maintain parity with main branch to ease backporting.
mridula-s109 pushed a commit that referenced this pull request Jun 2, 2025
* Rename and encapsulate InitializeArgs

* Move ElasticsearchEntitlementChecker out of api package.

It's an implementation detail that doesn't need to be exposed to the rest of
the system.

* Stub TestPathLookup (not yet implemented)
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jun 3, 2025
* Rename and encapsulate InitializeArgs

* Move ElasticsearchEntitlementChecker out of api package.

It's an implementation detail that doesn't need to be exposed to the rest of
the system.

* Stub TestPathLookup (not yet implemented)
joshua-adams-1 pushed a commit to joshua-adams-1/elasticsearch that referenced this pull request Jun 3, 2025
* Rename and encapsulate InitializeArgs

* Move ElasticsearchEntitlementChecker out of api package.

It's an implementation detail that doesn't need to be exposed to the rest of
the system.

* Stub TestPathLookup (not yet implemented)
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/elasticsearch that referenced this pull request Jun 5, 2025
* Rename and encapsulate InitializeArgs

* Move ElasticsearchEntitlementChecker out of api package.

It's an implementation detail that doesn't need to be exposed to the rest of
the system.

* Stub TestPathLookup (not yet implemented)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >refactoring Team:Core/Infra Meta label for core/infra team v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants